Deprecate usage of should_not raise_error
#50
Conversation
spec/expectations/should_spec.rb
Outdated
| @@ -5,7 +5,7 @@ | |||
| before :all do | |||
| path = RbConfig::CONFIG['bindir'] | |||
| exe = RbConfig::CONFIG['ruby_install_name'] | |||
| file = File.dirname(__FILE__) + '/should.rb' | |||
| file = File.dirname(__FILE__) + '/should.rb 2>&1' | |||
There was a problem hiding this comment.
It prevents running MSpec specs on Windows. Not sure whether it's important.
It would be a little more difficult to check STDERR of the command in this test. Another option is to write a deprecation message to STDOUT instead of STDERR.
There was a problem hiding this comment.
I think 2>&1 might actually work on Windows, could you move it to the line below though, so file is still an actual file?
andrykonchin
force-pushed
the
deprecate-should-not-raise-error
branch
from
eafaf6e
to
95a2dcf
Compare
lib/mspec/expectations/should.rb
Outdated
| @@ -23,6 +23,10 @@ def should_not(matcher = NO_MATCHER_GIVEN) | |||
| raise "should_not outside example" unless state | |||
| MSpec.actions :expectation, state | |||
|
|
|||
| if RaiseErrorMatcher === matcher | |||
| $stderr.puts "\nDeprecation: ->{}.should_not raise_error breaks code style and is deprecated" | |||
There was a problem hiding this comment.
We should show the caller here, so it's easy to know where it comes from.
There was a problem hiding this comment.
Could you use MSpec.deprecate (
mspec/lib/mspec/utils/deprecate.rb
Line 2 in 47e3cbf
That will automatically include the caller.
There was a problem hiding this comment.
replacement can be e.g. "a matcher to verify the result"
There was a problem hiding this comment.
Yeah, will try. I saw this helper but decided not to use it because it has a little bit different purpose - to notify about changing in API or DSL. But here we have a code style issue.
Nevertheless it should work well for our case.
spec/expectations/should_spec.rb
Outdated
| @@ -5,7 +5,7 @@ | |||
| before :all do | |||
| path = RbConfig::CONFIG['bindir'] | |||
| exe = RbConfig::CONFIG['ruby_install_name'] | |||
| file = File.dirname(__FILE__) + '/should.rb' | |||
| file = File.dirname(__FILE__) + '/should.rb 2>&1' | |||
There was a problem hiding this comment.
I think 2>&1 might actually work on Windows, could you move it to the line below though, so file is still an actual file?
|
I think we should start a PR on ruby/spec to fix the warnings, because if we merge and there are hundreds of warnings it will be very annoying for all people running ruby/spec. |
|
There are about 180 examples to change. Will try to fix them soon. |
Print deprecation message for every example of
-> {}.should_not raise_errorin specs. I believe it's enough to ensure that new specs don't contain deprecated DSL.Related issues:
Example